Skip to content

fix!: Refactor ConvertDocumentResponse for polymorphic deserialization based on docling-serve API response#388

Merged
edeandrea merged 4 commits intodocling-project:mainfrom
ArpanKrChakraborty:feature/convert-api
Mar 18, 2026
Merged

fix!: Refactor ConvertDocumentResponse for polymorphic deserialization based on docling-serve API response#388
edeandrea merged 4 commits intodocling-project:mainfrom
ArpanKrChakraborty:feature/convert-api

Conversation

@ArpanKrChakraborty
Copy link
Contributor

fix: Refactor ConvertDocumentResponse for polymorphic deserialization (#206)

Refactored ConvertDocumentResponse into a sealed abstract base class with
three concrete implementations (InBodyConvertDocumentResponse,
PreSignedUrlConvertDocumentResponse, ZipArchiveConvertDocumentResponse)
to properly handle different response types from Docling Serve API.

  • Use Jackson @JsonTypeInfo with DEDUCTION for automatic type detection of JSON-based responses
  • Leverage Java 17 sealed classes for type safety
  • Update reference implementation in docling-serve-client to handle these new types
  • Add comprehensive test coverage for all response types
  • Update documentation and Javadoc

Fixes #206

@ArpanKrChakraborty ArpanKrChakraborty force-pushed the feature/convert-api branch 2 times, most recently from e971bc7 to a5ea31c Compare March 9, 2026 08:51
@edeandrea
Copy link
Contributor

Hi @ArpanKrChakraborty sorry for the delay in getting to this - I've been out of the office for 2 weeks and I'm still catching up. I will get to this!

@github-actions
Copy link

github-actions bot commented Mar 11, 2026

:java_duke: JaCoCo coverage report

Overall Project 47.29% 🔴

There is no coverage information present for the Files changed

@github-actions
Copy link

github-actions bot commented Mar 11, 2026

TestsPassed ✅SkippedFailed
Gradle Test Results (all modules & JDKs)1002 ran1002 passed0 skipped0 failed
TestResult
No test annotations available

@github-actions
Copy link

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

@ArpanKrChakraborty ArpanKrChakraborty force-pushed the feature/convert-api branch 2 times, most recently from 52fbd90 to d74afef Compare March 11, 2026 09:06
@github-actions
Copy link

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

@edeandrea
Copy link
Contributor

Hi @ArpanKrChakraborty I do have to ask this question and please be honest.

How much of the initial description you provided on #206 and how much of the implementation in this PR is generated by AI?

@ArpanKrChakraborty
Copy link
Contributor Author

ArpanKrChakraborty commented Mar 12, 2026

Hi @edeandrea, in #206 the design changes I proposed are completely from my side. I went through docling-serve, docling-jobkit repos and the docling-serve documentation , to understand the flow and realized when which type of response is returned and proposed a basic superclass - subclass solution to it.

Regarding implementation, I did use AI to generate java docs to some extent and explore knowledge on jackson annotations (like a form of advanced google search) and such and that's it. What you see is a squashed commit, the implemention you see now is a result of many iteration of changes - I did like around 10-15 commits in total and this is the final result.

@edeandrea
Copy link
Contributor

Thank you! I will be back in the office tomorrow after a 2 week hiatus so will review then. Thank you for your patience.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors ConvertDocumentResponse in docling-serve-api into a sealed, polymorphic response hierarchy to support multiple Docling Serve convert response shapes (in-body JSON, presigned-url stats, and ZIP archive streams), and updates the reference client, docs, and tests accordingly.

Changes:

  • Replaced the former monolithic ConvertDocumentResponse model with a sealed base type plus InBodyConvertDocumentResponse, PreSignedUrlConvertDocumentResponse, and ZipArchiveConvertDocumentResponse.
  • Updated docling-serve-client to handle ZIP streaming responses and task-result responses that may be JSON or ZIP.
  • Updated documentation snippets and expanded tests to cover the new response types and ZIP behaviors (including referenced image artifacts).

Reviewed changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
docs/src/doc/docs/testcontainers.md Updates docs sample to use InBodyConvertDocumentResponse.
docs/src/doc/docs/getting-started.md Updates getting-started sample to use InBodyConvertDocumentResponse.
docs/src/doc/docs/docling-serve/serve-client.md Updates client docs; adds note for in-body response type (but error-handling snippet needs further fixes).
docs/src/doc/docs/docling-serve/serve-api.md Updates API docs to describe the three response variants.
docling-testing/docling-version-tests/.../TagsTester.java Updates version tests to branch on response type.
docling-serve/docling-serve-client/.../AbstractDoclingServeClientTests.java Expands client tests to assert response types and ZIP contents.
docling-serve/docling-serve-client/.../util/package-info.java Adds @NullMarked to the util package.
docling-serve/docling-serve-client/.../util/Utils.java Adds helper parsing for Content-Disposition and Content-Type.
docling-serve/docling-serve-client/.../operations/TaskOperations.java Adds Content-Type-based JSON-vs-ZIP handling for task results.
docling-serve/docling-serve-client/.../operations/StreamResponse.java Introduces a stream response wrapper for binary bodies + headers.
docling-serve/docling-serve-client/.../operations/HttpOperations.java Extends HTTP abstraction to support stream responses and readValue.
docling-serve/docling-serve-client/.../operations/ConvertOperations.java Adds ZIP streaming path for zip/multi-source conversions.
docling-serve/docling-serve-client/.../DoclingServeClient.java Implements stream GET/POST execution and StreamResponse handling.
docling-serve/docling-serve-api/.../convert/response/*.java Adds new response subtypes + ResponseType enum; refactors base type to sealed + Jackson deduction.
docling-serve/docling-serve-api/.../ConvertDocumentResponseTests.java Updates tests for the new response types.
docling-serve/docling-serve-api/.../DoclingServeTaskApi.java Updates Javadoc to reflect task result semantics.
docling-serve/docling-serve-api/.../DoclingServeConvertApi.java Updates Javadoc to reflect polymorphic convert responses.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

@edeandrea edeandrea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also didn't notice anything in the whats-new.md file describing this as a breaking change?

@edeandrea edeandrea changed the title fix: Refactor ConvertDocumentResponse for polymorphic deserialization based on docling-serve API response fix!: Refactor ConvertDocumentResponse for polymorphic deserialization based on docling-serve API response Mar 13, 2026
@edeandrea
Copy link
Contributor

Hi @ArpanKrChakraborty thank you for your patience! I've had copilot go through this. I've reviewed copilot's findings and I agree with them. I've also added some of my own findings.

@ArpanKrChakraborty
Copy link
Contributor Author

Hi @edeandrea. You want me to consider all of copilot's review points ? Also regarding the mentioning of breaking changes in whats-new.md, I was not sure how to put it, but let me draft up something for starters. Thanks for the review !

@edeandrea
Copy link
Contributor

Hi @edeandrea. You want me to consider all of copilot's review points ? Also regarding the mentioning of breaking changes in whats-new.md, I was not sure how to put it, but let me draft up something for starters. Thanks for the review !

Yes please. I've gone through everything copilot mentioned and they are things I would have mentioned myself (it even found some things I didn't during my manual review).

@edeandrea
Copy link
Contributor

You don't have to use the solution it proposed specifically, but the issues it found are valid issues.

@ArpanKrChakraborty
Copy link
Contributor Author

Thanks @edeandrea! I will refactor the code as suggested.

@edeandrea edeandrea force-pushed the feature/convert-api branch from 9fc28a9 to 0fef244 Compare March 13, 2026 18:11
@github-actions
Copy link

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

@github-actions
Copy link

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

@ArpanKrChakraborty
Copy link
Contributor Author

@edeandrea made the suggested enhancements. In addition to this I removed the util package I added in docling-serve-client. I felt utility functions in that util class are best placed under StreamResponse.ResponseHeaders to make it more cohesive, hence moved it there. I also updated the whats-new.md file with a description of the breaking-changes made in this PR.

@ArpanKrChakraborty
Copy link
Contributor Author

@edeandrea I have a little concern regarding how S3Target type ConvertDocumentRequest are handled. I went through the docling-jobkit code, and I understand how the CLI handles it but docling-serve makes use of the orchestrators and I am not sure how its handled in that case (I didn't see any S3Target handling for local / redis orchestrator, only usage is in kfp orchestrator and I am unable to understand that).

I have mentioned in the javadoc (made an assumption) that for S3Target the conversion results are archived and pushed to the provided COS bucket as this is the case with PutTarget, but I am not exactly sure about when it comes to S3Target. Let me know if you want me to remove this part of the javadoc, cause it may not be accurate.

@github-actions
Copy link

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

ArpanKrChakraborty and others added 3 commits March 17, 2026 16:52
…docling-project#206)

Refactored ConvertDocumentResponse into a sealed abstract base class with
three concrete implementations (InBodyConvertDocumentResponse,
PreSignedUrlConvertDocumentResponse, ZipArchiveConvertDocumentResponse)
to properly handle different response types from Docling Serve API.

- Use Jackson @JsonTypeInfo with DEDUCTION for automatic type detection of JSON-based responses
- Leverage Java 17 sealed classes for type safety
- Update reference implementation in docling-serve-client to handle these new types
- Add comprehensive test coverage for all response types
- Update documentation and Javadoc
- Apply Spotless

Fixes docling-project#206

Signed-off-by: Arpan Chakraborty <arpan.chakraborty1908@gmail.com>
- Removed previously added util package in docling-serve-client.
- Moved the utility functions from the Utils class under docling-serve-client to StreamResponse.ResponseHeaders as defaults
- Updated whats-new.md to document breaking changes in this release.

Signed-off-by: Arpan Chakraborty <Arpan.Chakraborty1908@gmail.com>
@edeandrea edeandrea force-pushed the feature/convert-api branch from 61d6ccb to 816e571 Compare March 17, 2026 20:52
@edeandrea
Copy link
Contributor

@ArpanKrChakraborty I went through everything - nice work! I added one more small comment.

I have a little concern regarding how S3Target type ConvertDocumentRequest are handled. I went through the docling-jobkit code, and I understand how the CLI handles it but docling-serve makes use of the orchestrators and I am not sure how its handled in that case (I didn't see any S3Target handling for local / redis orchestrator, only usage is in kfp orchestrator and I am unable to understand that).

I have mentioned in the javadoc (made an assumption) that for S3Target the conversion results are archived and pushed to the provided COS bucket as this is the case with PutTarget, but I am not exactly sure about when it comes to S3Target. Let me know if you want me to remove this part of the javadoc, cause it may not be accurate.

I would just leave it how it was for now. To be honest I'm not 100% sure how it works either, so lets not introduce any uncertainty.

@github-actions
Copy link

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

- Update getting-started.md

Signed-off-by: Arpan Chakraborty <Arpan.Chakraborty1908@gmail.com>
@ArpanKrChakraborty
Copy link
Contributor Author

@edeandrea pushed the requested changes.

@edeandrea
Copy link
Contributor

Thanks very much for this @ArpanKrChakraborty ! once CI checks run I'll merge.

Thank you for your patience and for the back and forth on this!

@github-actions
Copy link

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

@edeandrea edeandrea merged commit 4ae819a into docling-project:main Mar 18, 2026
21 checks passed
@edeandrea
Copy link
Contributor

@all-contributors add @ArpanKrChakraborty for code, test, content, doc

@allcontributors
Copy link
Contributor

@edeandrea

I've put up a pull request to add @ArpanKrChakraborty! 🎉

@docling-java-ops
Copy link
Contributor

🎉 This issue has been resolved in v0.5.0 (Release Notes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released Issue has been released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

image_export_mode=referenced returns image paths, but image files are missing

3 participants